-
Notifications
You must be signed in to change notification settings - Fork 0
Added user authentication with email and password #46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements user authentication with email and password using Supabase, adding sign-in, sign-up, and email confirmation flows. However, there are several critical issues that need to be addressed, including incorrect file naming for Next.js middleware, incorrect import paths, missing error pages, and security vulnerabilities.
Key Changes
- Added Supabase authentication functions for email/password sign-up and sign-in
- Created a temporary login page with forms for testing authentication
- Implemented email confirmation route handler for Supabase OTP verification
- Added middleware (incorrectly named as proxy.ts) for session management and route protection
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/proxy.ts | Middleware entry point with incorrect naming and import path - should be middleware.ts |
| src/lib/client/supabase/proxy.ts | Session management middleware logic for protecting routes |
| src/lib/client/supabase/client.ts | Browser client initialization for Supabase |
| src/lib/client/supabase/auth.ts | Authentication functions for email/password sign-up and sign-in |
| src/app/login/page.tsx | Temporary UI for testing authentication flows |
| src/app/auth/confirm/route.ts | Email confirmation handler with open redirect vulnerability |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,20 @@ | |||
| import { updateSession } from "@/lib/supabase/proxy"; | |||
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import path is incorrect. The file proxy.ts is located at src/lib/client/supabase/proxy.ts, not @/lib/supabase/proxy. This should be @/lib/client/supabase/proxy to match the actual file location.
| import { updateSession } from "@/lib/supabase/proxy"; | |
| import { updateSession } from "@/lib/client/supabase/proxy"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double check this
| import { updateSession } from "@/lib/supabase/proxy"; | ||
| import { type NextRequest } from "next/server"; | ||
|
|
||
| export async function proxy(request: NextRequest) { |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should be named middleware.ts rather than proxy.ts and placed at the root of src/ directory. Next.js expects middleware to be exported from a file named middleware.ts at the root of the src directory (or project root). The exported function should also be named middleware instead of proxy.
| export async function proxy(request: NextRequest) { | |
| export async function middleware(request: NextRequest) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the name proxy.ts for this file work? Because for the Next.js version (15.5.6) we're using it asks for the function to be called middleware. The newest version (16) of Next.js requires proxy.ts instead of middleware.ts, but in the version we're using this isn't used yet.
| export async function signInWithEmail(email: string, password: string) { | ||
| const supabase = createClient(); | ||
| const { data, error } = await supabase.auth.signInWithPassword({ | ||
| email, | ||
| password, | ||
| }); | ||
| return { data, error }; | ||
| } |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding email format validation before calling the Supabase API. While the HTML input type="email" provides basic validation, additional checks in the auth functions would provide defense-in-depth and ensure consistent validation if these functions are called from other contexts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can ignore this for now for the same reason that we ignore the password checks. Performing these strength requirements checks on the frontend would provide faster feedback to the user.
| const next = searchParams.get('next') ?? '/' | ||
|
|
||
| if (token_hash && type) { | ||
| const supabase = await createClient() | ||
|
|
||
| const { error } = await supabase.auth.verifyOtp({ | ||
| type, | ||
| token_hash, | ||
| }) | ||
| if (!error) { | ||
| // redirect user to specified redirect URL or root of app | ||
| redirect(next) |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The next parameter from search params is used directly for redirection without validation. This creates an open redirect vulnerability where attackers could craft malicious links to redirect users to external sites after authentication. Validate that the redirect URL is relative or on the same domain before using it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good practice
|
|
||
| export async function signUpWithEmail(email: string, password: string) { | ||
| const supabase = createClient(); | ||
| const { data, error } = await supabase.auth.signUp({ email, password }); |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The signUpWithEmail function should include an emailRedirectTo option to specify where users should be redirected after confirming their email. Without this, the confirmation link may not work correctly with the /auth/confirm route. Consider adding: options: { emailRedirectTo: ${window.location.origin}/auth/confirm }
| const { data, error } = await supabase.auth.signUp({ email, password }); | |
| const { data, error } = await supabase.auth.signUp( | |
| { email, password }, | |
| { emailRedirectTo: `${window.location.origin}/auth/confirm` } | |
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the confirmation link work properly?
| import { type EmailOtpType } from '@supabase/supabase-js' | ||
| import { type NextRequest } from 'next/server' | ||
|
|
||
| import { createClient } from '@/lib/client/supabase/server' | ||
| import { redirect } from 'next/navigation' |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent quote style. This file uses single quotes while other files in the codebase use double quotes (e.g., src/lib/client/supabase/client.ts, src/lib/client/supabase/auth.ts). Consider using double quotes for consistency with the rest of the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is also the errors Vercel points out. Make sure you're up-to-date with the latest main branch! There's code in the main branch that runs formatting/linting automatically for you before every commit.
| } | ||
|
|
||
| // redirect the user to an error page with some instructions | ||
| redirect('/auth/auth-code-error') |
Copilot
AI
Nov 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The route redirects to /auth/auth-code-error but this page doesn't exist in the codebase. This will result in a 404 error when email confirmation fails. You need to create the error page at src/app/auth/auth-code-error/page.tsx.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just create a very basic page
LeandroHamaguchi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was one of the hardest tickets, and overall, great work Patricia!
| } | ||
|
|
||
| // redirect the user to an error page with some instructions | ||
| redirect('/auth/auth-code-error') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just create a very basic page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use a different name for this file since it's the same as src/proxy.ts
| import { type EmailOtpType } from '@supabase/supabase-js' | ||
| import { type NextRequest } from 'next/server' | ||
|
|
||
| import { createClient } from '@/lib/client/supabase/server' | ||
| import { redirect } from 'next/navigation' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is also the errors Vercel points out. Make sure you're up-to-date with the latest main branch! There's code in the main branch that runs formatting/linting automatically for you before every commit.
| const next = searchParams.get('next') ?? '/' | ||
|
|
||
| if (token_hash && type) { | ||
| const supabase = await createClient() | ||
|
|
||
| const { error } = await supabase.auth.verifyOtp({ | ||
| type, | ||
| token_hash, | ||
| }) | ||
| if (!error) { | ||
| // redirect user to specified redirect URL or root of app | ||
| redirect(next) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good practice
|
|
||
| export async function signUpWithEmail(email: string, password: string) { | ||
| const supabase = createClient(); | ||
| const { data, error } = await supabase.auth.signUp({ email, password }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the confirmation link work properly?
| @@ -0,0 +1,20 @@ | |||
| import { updateSession } from "@/lib/supabase/proxy"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double check this
Added user authentication with email and password. For now the tmp page displays the user token when signed in successfully.
